Disclosure: This review was drafted with AI assistance (Claude Code) to help trace the dispose lifecycle and identify edge cases.

Thanks for this PR @nochev — the leak you identified is real and we've been wanting this fixed. I've spent some time testing and reviewing the approach and found a few issues that need addressing before this can merge. Happy to help work through them.

1. Double-dispose crash (this is what's failing CI)

Registering this.dispose as the player's 'dispose' handler causes TrackButton.dispose() to run twice during player teardown — once via the event, and again via the normal child disposal chain. The isDisposed_ guard in Component.dispose() would catch this, but your dispose() override does work before calling super.dispose(), so on the second call this.player_ is already null:

TypeError: Cannot read properties of null (reading 'off')
    at AudioTrackButton.dispose
    at ControlBar.dispose
    at Player.dispose

The sequence is:

1. Player.dispose() → Component.dispose() fires 'dispose' event
2. disposeHandler fires → TrackButton.dispose() → super.dispose() → sets this.player_ = null
3. Player continues disposing children → ControlBar → AudioTrackButton
4. TrackButton.dispose() runs again → this.player_.off(...) → 💥

Fix: Use a separate cleanup method instead of wiring the full dispose() to the player's 'dispose' event.

2. Events.off(elem, type, null) removes ALL listeners

This is subtle. In video.js, calling removeEventListener(type, null) (or any falsy fn) hits this path in utils/events.js:

// If no listener was provided, remove all listeners for type
if (!fn) {
    removeType(elem, type);
    return;
}

If you null out the handler references before the second cleanup call (which happens via the child disposal chain as described above), tracks.removeEventListener('removetrack', null) nukes every removetrack listener on the emulated TextTrackList — including the native-to-emulated proxy set up in Html5.proxyNativeTracksForType_(), and any listeners from other consumers. Since player.textTracks() delegates to tech.textTracks() (same object), this affects the entire track event pipeline.

Fix: Guard removeEventListener calls so they only run when the handler reference is still set.

3. Missing null guard on tracks in dispose (minor)

The constructor early-returns when !tracks, meaning updateHandler_ / disposeHandler_ are never created. But dispose() unconditionally accesses this.options_.tracks. In practice textTracks() / audioTracks() always return a TrackList, but for defensive consistency the guard should match the constructor.

Suggested implementation

Here's an approach that addresses all three issues — tested locally with full Safari and Chrome passes:

constructor(player, options) {
    const tracks = options.tracks;

    super(player, options);

    if (this.items.length <= 1) {
      this.hide();
    }

    if (!tracks) {
      return;
    }

    this.updateHandler_ = Fn.bind_(this, this.update);
    this.cleanupHandler_ = Fn.bind_(this, this.cleanupTrackListeners_);

    tracks.addEventListener('removetrack', this.updateHandler_);
    tracks.addEventListener('addtrack', this.updateHandler_);
    tracks.addEventListener('labelchange', this.updateHandler_);
    this.player_.on('ready', this.updateHandler_);
    this.player_.on('dispose', this.cleanupHandler_);
  }

  /**
   * Remove track list event listeners.
   *
   * @private
   */
  cleanupTrackListeners_() {
    const tracks = this.options_.tracks;

    if (tracks && this.updateHandler_) {
      tracks.removeEventListener('removetrack', this.updateHandler_);
      tracks.removeEventListener('addtrack', this.updateHandler_);
      tracks.removeEventListener('labelchange', this.updateHandler_);
    }

    if (this.player_) {
      if (this.updateHandler_) {
        this.player_.off('ready', this.updateHandler_);
      }
      if (this.cleanupHandler_) {
        this.player_.off('dispose', this.cleanupHandler_);
      }
    }

    this.updateHandler_ = null;
    this.cleanupHandler_ = null;
  }

  /**
   * Dispose of the Component and remove all event listeners.
   *
   * @override
   */
  dispose() {
    this.cleanupTrackListeners_();
    super.dispose();
  }

The key ideas:

• Separate cleanup method — the player's 'dispose' handler only cleans up listeners, it doesn't trigger a full component disposal
• Null guards on handler references — prevents the Events.off nuke-all-listeners footgun
• = null instead of delete — matches the convention in Component.dispose()

Happy to answer any questions on any of this. The original diagnosis of the leak was spot on — these are just implementation details to make it safe in video.js's dispose lifecycle.